Add new invalid_markdown_table rustdoc lint - #159583

Applied comments.

2nd try. Let's do an FCP.

//! | col1 |
//! | ---- |
//! | `code_with(|arg| arg)` |

error: table row has too many columns
  --> $DIR/unescaped_pipe_in_table_cell.rs:5:18
   |
5  | //! | `code_with(|arg| arg)` |
   |                  ^ help: any content after this column divider is discarded
   |
   = help: to escape `|` characters in tables, add a `\` before them like `\|`
note: the lint level is defined here
  --> $DIR/unescaped_pipe_in_table_cell.rs:1:9
   |
1  | #![deny(rustdoc::unescaped_pipe_in_table_cell)]
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

@rfcbot merge rustdoc-internals

@Urgau has proposed to merge this. The next step is review by the rest of the tagged team members:

• @GuillaumeGomez
• @Manishearth
• @Urgau
• @camelid
• @fmease
• @lolbinarycat
• @notriddle
• @yotamofek

Concerns:

• overly specific lint name resolved by Add new invalid_markdown_table rustdoc lint #159583 (comment)

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

Minor nits about the implementation, but massive approval for the lint in concept!

Definitely in favor of the lint itself, but I wonder if we should have a more generic lint name and then we group together this and any related (current or future) lints about Markdown gotchas? E.g. we have rustdoc::invalid_html_tags that groups together anything related to that, including (soon: #158709) issues when HTML is interleaved with Markdown.

For example, it could be rustdoc::invalid_markdown, or rustdoc::markdown_gotchas, or a Better Name (TM). I could even imagine us renaming rustdoc::invalid_html_tags to something more general like rustdoc::invalid_markdown and including this as part of it.

On a related note, my feeling with #158709 is that additions of new cases to an existing warn-by-default lint do not require FCP since they are not part of stability guarantees, but please let me know if you disagree.

@rfcbot reviewed
@rfcbot concern overly specific lint name

@camelid: Very fair point. I think having a lint group for all such lints is a good idea. As for the current lint name, I'll rename it as invalid_markdown_table.

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Finished benchmarking commit (ff4bc60): comparison URL.

Overall result: ❌ regressions - please read:

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

• If the regression was expected or you think it can be justified,
  please write a comment with sufficient written justification, and add
  @rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
• If you think that you know of a way to resolve the regression, try to create
  a new PR with a fix for the regression.
• If you do not understand the regression or you think that it is just noise,
  you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
  were already notified of this PR).

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

Max RSS (memory usage)

Cycles

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 471.419s -> 469.832s (-0.34%)
Artifact size: 400.34 MiB -> 400.27 MiB (-0.02%)

Caused the perf regression in the rollup.
This does more work because it's a new lint, and the regression is quite minor, so we probably just have to accept this
@rustbot label: +perf-regression-triaged